Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint: Require # Safety section in pub unsafe fn docs #4535

Merged
merged 1 commit into from
Sep 19, 2019
Merged

Conversation

llogiq
Copy link
Contributor

@llogiq llogiq commented Sep 11, 2019

changelog: add missing_safety_doc lint

This fixes #2207

clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
clippy_lints/src/doc.rs Show resolved Hide resolved
tests/ui/doc_unsafe.rs Outdated Show resolved Hide resolved
@flip1995 flip1995 added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Sep 12, 2019
@llogiq llogiq force-pushed the unsafe-doc branch 5 times, most recently from 692fea2 to 6b6bdb8 Compare September 12, 2019 19:12
@llogiq
Copy link
Contributor Author

llogiq commented Sep 12, 2019

Thanks for the review!

r? @flip1995

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two NITs in the doc, then this should be ready to go

clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
pub unsafe fn apocalypse(universe: &mut ()) {
unimplemented!();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a test that the lint doesn't trigger on non-async functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean an async function? How would that work?

Copy link
Member

@phansch phansch Sep 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sorry, I meant adding a test for a non-unsafe function :D

@llogiq
Copy link
Contributor Author

llogiq commented Sep 13, 2019

Thanks again, @flip1995

@flip1995
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 16, 2019

📌 Commit 38f252a has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Sep 16, 2019

⌛ Testing commit 38f252a with merge 2ff1202...

bors added a commit that referenced this pull request Sep 16, 2019
New lint: Require `# Safety` section in pub unsafe fn docs

changelog: add `missing_safety_doc` lint

This fixes #2207
@bors
Copy link
Collaborator

bors commented Sep 16, 2019

💔 Test failed - checks-travis

@flip1995
Copy link
Member

@bors rollup

@flip1995
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 19, 2019

📌 Commit 70a7dab has been approved by flip1995

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Sep 19, 2019
New lint: Require `# Safety` section in pub unsafe fn docs

changelog: add `missing_safety_doc` lint

This fixes rust-lang#2207
bors added a commit that referenced this pull request Sep 19, 2019
Rollup of 4 pull requests

Successful merges:

 - #4511 (New lint: mem_replace_with_uninit)
 - #4535 (New lint: Require `# Safety` section in pub unsafe fn docs)
 - #4539 (Changes cast-lossless to a pedantic lint)
 - #4544 (#4542 remove machine applicable suggestion)

Failed merges:

r? @ghost

changelog: none
bors added a commit that referenced this pull request Sep 19, 2019
New lint: Require `# Safety` section in pub unsafe fn docs

changelog: add `missing_safety_doc` lint

This fixes #2207
@bors
Copy link
Collaborator

bors commented Sep 19, 2019

⌛ Testing commit 70a7dab with merge f783ff3...

@bors
Copy link
Collaborator

bors commented Sep 19, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing f783ff3 to master...

@bors bors merged commit 70a7dab into master Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rust API Guideline: Unsafe functions are documented with a "Safety" section
4 participants